Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the LSP independent of level assignment. #186

Open
wants to merge 2 commits into
base: updated_enabled_cdot
Choose a base branch
from

Conversation

kape1395
Copy link
Collaborator

The fingerprint calculation reverted to is_const (as it is now done in the main branch) instead of get_level to make the fingerprint calculation independent of the level assignment. This is done to avoid having level assignments on the critical path in the LSP, as it is too slow. In my case, it took 30 seconds to calculate the levels, which blocks subsequent LSP commands.

@ahelwer
Copy link
Collaborator

ahelwer commented Dec 24, 2024

The TLAPM parser has a level-checker?

@kape1395
Copy link
Collaborator Author

It has, but I'm unsure how complete it is. See e.g. https://github.com/tlaplus/tlapm/blob/d8e2a2135063fbb0bd6a4fb3af7bba46951059c8/src/expr/e_levels.ml

@ahelwer
Copy link
Collaborator

ahelwer commented Dec 24, 2024

Would you leave your thoughts on tlaplus/rfcs#16 since you know much more about the TLAPM parser than anyone else?

@kape1395
Copy link
Collaborator Author

Thanks for starting that discussion. I will try to join it, but my knowledge of that parser is limited.

The fingerprint calculation reverted to is_const (as it is now done in the main branch) instead of get_level to make the fingerprint calculation independent of the level assignment. This is done to avoid having level assignments on the critical path in the LSP, as it is too slow. In my case, it took 30 seconds to calculate the levels, which blocks subsequent LSP commands.

Signed-off-by: Karolis Petrauskas <[email protected]>
@kape1395 kape1395 force-pushed the updated_enabled_cdot-fix-lsp-perf branch from d8e2a21 to 0ae62a0 Compare December 30, 2024 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants